Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Modifie DeleteImages (préparation pour Django 4) #6574

Merged
merged 1 commit into from
Jan 14, 2024
Merged

Modifie DeleteImages (préparation pour Django 4) #6574

merged 1 commit into from
Jan 14, 2024

Conversation

Arnaud-D
Copy link
Contributor

Dans Django 4, DeleteView change de comportement. Notre manière d'utiliser cette vue générique était un peu exotique et ne correspond pas au cas d'usage prévu.

Je l'ai changé pour une vue normale en reprenant le comportement actuel.

On ne peut plus appeler la vue avec la méthode DELETE, mais ce n'a jamais été utilisé réellement. Le formulaire HTML utilise POST et l'ancien code redirigeait post() vers delete(). Le nouveau code est finalement plus direct.

Contrôle qualité

Vérifier que la suppression d'image dans la liste d'image de la galerie fonctionne bien.

Vérifier que la suppression d'images depuis la page d'une image donnée fonctionne bien.

@Arnaud-D Arnaud-D added the C-Back Concerne le back-end Django label Jan 14, 2024
@Arnaud-D Arnaud-D force-pushed the prepa_django4_deleteimages branch from bb31d33 to af529e1 Compare January 14, 2024 19:42
@coveralls
Copy link

coveralls commented Jan 14, 2024 •
edited
Loading

Coverage Status

coverage: 88.811%. remained the same
when pulling af529e1 on Arnaud-D:prepa_django4_deleteimages
into 411bc88 on zestedesavoir:dev.

@Arnaud-D Arnaud-D changed the title Modifie DeleteImages en préparation pour Django 4 Modifie DeleteImages (préparation pour Django 4) Jan 14, 2024
Situphen
Situphen approved these changes Jan 14, 2024
View reviewed changes
Copy link
Member

@Situphen Situphen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA OK Ça fonctionne bien comme prévu en local

@Situphen Situphen merged commit 9b93b43 into zestedesavoir:dev Jan 14, 2024
12 checks passed
@philippemilink
Copy link
Member

philippemilink commented Jan 14, 2024 •
edited
Loading

DeleteView est aussi utilisé ailleurs :

% git grep -n DeleteView zds/
zds/featured/tests/tests.py:243:class FeaturedResourceDeleteViewTest(TestCase):
zds/featured/tests/tests.py:271:class FeaturedResourceListDeleteViewTest(TestCase):
zds/featured/views.py:9:from django.views.generic import CreateView, RedirectView, UpdateView, FormView, DeleteView
zds/featured/views.py:192:class FeaturedResourceDeleteDetail(FeaturedViewMixin, DeleteView):
zds/featured/views.py:338:class FeaturedMessageDelete(FeaturedViewMixin, DeleteView):
zds/tutorialv2/views/containers_extracts.py:11:from django.views.generic import DeleteView, FormView
zds/tutorialv2/views/containers_extracts.py:228:class DeleteContainerOrExtract(LoggedWithReadWriteHability, SingleContentViewMixin, DeleteView):
zds/tutorialv2/views/contents.py:13:from django.views.generic import DeleteView
zds/tutorialv2/views/contents.py:280:class DeleteContent(LoginRequiredMixin, SingleContentViewMixin, DeleteView):

Il n'y a pas besoin de changer aussi à ces endroits-là ?

@Arnaud-D
Copy link
Contributor Author

Arnaud-D commented Jan 14, 2024 •
edited
Loading

Oui, il faut aussi examiner ces cas-là. Je vais regarder au cas par cas, parce que la solution n'est pas toujours la même, et dans certains cas, ça devra peut-être être fait en même temps que la bascule sur la nouvelle version. Ça ne sera pas possible de le faire en amont.

@Arnaud-D Arnaud-D deleted the prepa_django4_deleteimages branch February 24, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Situphen Situphen Situphen approved these changes

Assignees
No one assigned
Labels
C-Back Concerne le back-end Django
Projects
Suivi des PR
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Arnaud-D @coveralls @philippemilink @Situphen